Skip to content

Repository navigation

feat: add require-read-r rule - #11

Open
nzakas wants to merge 3 commits into
rule/no-useless-catfrom
rule/require-read-r
Open

feat: add require-read-r rule#11
nzakas wants to merge 3 commits into
rule/no-useless-catfrom
rule/require-read-r

Conversation

@nzakas

@nzakas nzakas commented Sep 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Adds shell/require-read-r, which mirrors ShellCheck SC2162: read without -r treats backslashes as escapes and mangles input.

Behavior

  • Reports read when none of its short-flag arguments before -- contains r. Combined flags like -rs and separate flags like -t 5 -r both count.
  • Autofix: inserts -r right after read (read -s pw → read -r -s pw).
  • Recommended config: "error".

Documentation

Adds docs/rules/require-read-r.md, following the format of the @eslint/json, @eslint/css, and @eslint/markdown rule docs: description, background, rule details with incorrect and correct examples, options, when not to use it, and the ShellCheck reference. The rule's meta.docs.url points at that file, and its README table entry links to it.

Testing

  • 8 RuleTester cases.
  • An end-to-end verifyAndFix test in tests/autofix.test.ts.
  • 5 documentation checks.

284 tests total; build, lint, and format checks pass.

🤖 Generated with Claude Code

Stack created with GitHub Stacks CLI • Give Feedback 💬

@coderabbitai

coderabbitai Bot commented Sep 18, 2026 •
edited
Loading

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 31 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 9359e815-4228-40e6-8b85-180447a5d472
📥 Commits

Reviewing files that changed from the base of the PR and between 5844ce2 and 52bd15b.

📒 Files selected for processing (7)
  • README.md
  • docs/rules/require-read-r.md
  • src/index.spec.ts
  • src/index.ts
  • src/rules/require-read-r.spec.ts
  • src/rules/require-read-r.ts
  • tests/autofix.test.ts
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@nzakas
nzakas added this pull request to stack #16 September 18, 2026 15:00
@nzakas
nzakas marked this pull request as ready for review September 18, 2026 15:01
@nzakas
nzakas force-pushed the rule/require-read-r branch 2 times, most recently from 97e8f63 to 6759d09 Compare September 22, 2026 19:56
@nzakas
nzakas force-pushed the rule/require-read-r branch from 6759d09 to 91fdea3 Compare September 22, 2026 20:23
@nzakas
nzakas force-pushed the rule/require-read-r branch 2 times, most recently from 85ef581 to be6c318 Compare September 22, 2026 21:21
@nzakas
nzakas requested a balanced review from Copilot September 28, 2026 18:25
Copilot AI reviewed Sep 28, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Option operands containing -r can suppress valid reports even though raw mode is not enabled.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Adds the recommended bash/require-read-r rule to prevent read from mangling backslashes.

Changes:

  • Implements and registers the autofixable rule.
  • Adds unit, integration, and documentation coverage.
  • Documents the rule and updates the README.
File Description
src/​rules/​require-read-r.ts Implements detection and autofixing.
src/​rules/​require-read-r.spec.ts Adds rule tests.
src/​index.ts Registers and recommends the rule.
src/​index.spec.ts Verifies rule registration.
tests/​autofix.test.ts Adds end-to-end autofix coverage.
docs/​rules/​require-read-r.md Documents behavior and usage.
README.md Adds the rule to the catalog.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/rules/require-read-r.ts Outdated
@nzakas

nzakas commented Sep 29, 2026

Copy link
Copy Markdown
Member Author

@copilot address review comments. Be sure to add tests.

Copilot AI commented Sep 29, 2026

Copy link
Copy Markdown

@copilot address review comments. Be sure to add tests.

Fixed in commit 7b4a7d1; added regression tests for separate and attached delimiter operands.

@nzakas
nzakas force-pushed the rule/require-read-r branch from 7b4a7d1 to 777ec05 Compare October 1, 2026 18:07
@nzakas
nzakas force-pushed the rule/require-read-r branch from dfc7b1a to d3ee679 Compare October 2, 2026 16:05
@nzakas
nzakas force-pushed the rule/require-read-r branch 2 times, most recently from 99a4eeb to 3b627dd Compare October 6, 2026 19:09
@nzakas
nzakas force-pushed the rule/require-read-r branch from 3b627dd to db65040 Compare October 6, 2026 19:26
nzakas and others added 3 commits October 6, 2026 15:35
@nzakas @claude
feat: add require-read-r rule
741b77f 
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@nzakas
fix: parse read option operands
c8e4d9f 
Co-authored-by: nzakas <38546+nzakas@users.noreply.github.com>
@nzakas @claude
fix: add languages and dialects to require-read-r meta
52bd15b 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@nzakas
nzakas force-pushed the rule/require-read-r branch from db65040 to 52bd15b Compare October 6, 2026 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

feature

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@nzakas